Fix incorrectly overriden static type-hint in Reader and AbstractCsv #285
Conversation
The type-hint says "@return static", which means if you call Reader::createFromPath() it should return a Reader, not an AbstractCsv. Without this you can't use any of the methods provided by Reader without getting warnings during static analysis.
|
@Jalle19 thanks for your PR. Indeed when 9.0.0 was released this bug on the return type of all the named constructors methods was introduced. The issue that I have is that your resolution may be considered a BC break by some. If it's not a BC break then you also need to fix the other named constructor return type (ie all the |
|
I guess I'll be fixing the other createFrom methods too then. I have a hard time coming up with a scenario where this would break anything, if I do I can mention it here. |
|
@Jalle19 TBH I too don't think it is BC break since removing the return type does not restrict or change the current public API. So IMHO we can move on with your PR once finalized ... unless someone disagree 👍 |
|
@nyamsprod I removed all the relevant static return type-hints |
|
let's merge this .. I'll release a new CSV patch version later in the week if I forgot just ping me here again 👍 |
|
Thanks! |
Introduction
The following code produces this Phan error:
PhanUndeclaredMethod Call to undeclared method \League\Csv\AbstractCsv::setHeaderOffset:Proposal
Remove the static return type-hint from
AbstractCsvsince sub-classes now overridecreateFromPath().Backward Incompatible Changes
None
Targeted release version
9.1.2
Open issues
There's some references to this in #266